Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Remove false-positive reflection calls in dataflow #18269

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Dec 11, 2024

When debugging a data flow performance issue, I noticed that we may have incorrect flow out of calls using reflection.

The data flow library uses the Dispatch.qll library to resolve calls, and this library distinguishes between the static target and dynamic targets of calls; for reflection-based calls, any method with matching name and arity is considered a static target, and this is then refined based on type information when calculating dynamic call targets. This means that "static target" of reflection calls is overly broad, and should not be used in data flow.

@github-actions github-actions bot added the C# label Dec 11, 2024
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Dec 12, 2024
@hvitved hvitved marked this pull request as ready for review December 12, 2024 09:36
@Copilot Copilot bot review requested due to automatic review settings December 12, 2024 09:36
@hvitved hvitved requested a review from a team as a code owner December 12, 2024 09:36

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (2)
  • csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll: Language not supported
  • csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hvitved hvitved merged commit 5d18e23 into github:main Dec 12, 2024
21 checks passed
@hvitved hvitved deleted the csharp/dataflow-reflection-call branch December 12, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants